fix: using Arabic numbers instead of Urdu numbers in Arabic localization#53
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can disable sequence diagrams in the walkthrough.Disable the |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/src/utils/utility.dart (1)
32-49: Consider extracting a shared helper to reduce duplication.Both
convertToUrduNumbersandconvertToArabicNumbersshare identical logic, differing only in the target digit array. A shared helper could reduce duplication if more numeral systems are added in the future.♻️ Optional refactor
/// Generic digit conversion helper String _convertToDigits(int input, List<String> targetDigits) { const english = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']; var result = input.toString(); for (var i = 0; i < english.length; i++) { result = result.replaceAll(english[i], targetDigits[i]); } return result; } String convertToUrduNumbers(int input) => _convertToDigits(input, ['۰', '۱', '۲', '۳', '۴', '۵', '۶', '۷', '۸', '۹']); String convertToArabicNumbers(int input) => _convertToDigits(input, ['٠', '١', '٢', '٣', '٤', '٥', '٦', '٧', '٨', '٩']);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/src/utils/utility.dart` around lines 32 - 49, Both convertToArabicNumbers and convertToUrduNumbers duplicate the same digit-replacement logic; extract a shared helper (e.g., _convertToDigits) that takes the input int and a targetDigits List<String>, move the english digits array and replacement loop into that helper, and update convertToArabicNumbers and convertToUrduNumbers to call _convertToDigits with their respective target digit arrays to remove duplication and centralize the conversion logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/src/utils/utility.dart`:
- Around line 42-48: The comments in the digit-replacement loop and return
statement incorrectly reference "Urdu"; update those comments to say "Arabic" so
they accurately describe the function that replaces English digits with Arabic
digits (references: the loop using english and arabic arrays and the return
statement returning result).
---
Nitpick comments:
In `@lib/src/utils/utility.dart`:
- Around line 32-49: Both convertToArabicNumbers and convertToUrduNumbers
duplicate the same digit-replacement logic; extract a shared helper (e.g.,
_convertToDigits) that takes the input int and a targetDigits List<String>, move
the english digits array and replacement loop into that helper, and update
convertToArabicNumbers and convertToUrduNumbers to call _convertToDigits with
their respective target digit arrays to remove duplication and centralize the
conversion logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d89ef9c4-5e63-495c-aef7-ab46833a56d9
📒 Files selected for processing (2)
lib/src/messages/languages/ar_msg.dartlib/src/utils/utility.dart
Updated the convertToArabicNumbers function to correctly replace English digits with Arabic counterparts.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/src/utils/utility.dart (1)
27-49: Extract the shared digit-conversion loop.
convertToArabicNumbers()is now a near copy ofconvertToUrduNumbers(). Pulling the common logic into a private helper would reduce drift and make future numeral-localization fixes cheaper.♻️ Proposed refactor
+String _convertDigits(int input, List<String> localizedDigits) { + const english = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']; + + var result = input.toString(); + for (var i = 0; i < english.length; i++) { + result = result.replaceAll(english[i], localizedDigits[i]); + } + + return result; +} + String convertToUrduNumbers(int input) { - // List of English digits - const english = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']; - - // List of corresponding Urdu digits const urdu = ['۰', '۱', '۲', '۳', '۴', '۵', '۶', '۷', '۸', '۹']; - - // Convert the input number to a string - var result = input.toString(); - - // Replace each English digit with its Urdu counterpart - for (var i = 0; i < english.length; i++) { - result = result.replaceAll(english[i], urdu[i]); - } - - // Return the result with Urdu digits - return result; + return _convertDigits(input, urdu); } String convertToArabicNumbers(int input) { - // List of English digits - const english = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']; - - // List of corresponding Arabic digits const arabic = ['٠', '١', '٢', '٣', '٤', '٥', '٦', '٧', '٨', '٩']; - - // Convert the input number to a string - var result = input.toString(); - - // Replace each English digit with its Arabic counterpart - for (var i = 0; i < english.length; i++) { - result = result.replaceAll(english[i], arabic[i]); - } - - // Return the result with Arabic digits - return result; + return _convertDigits(input, arabic); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/src/utils/utility.dart` around lines 27 - 49, The digit-replacement loop in convertToArabicNumbers is duplicated in convertToUrduNumbers; extract that shared logic into a private helper (e.g., _convertDigits or _replaceDigits) that accepts the input string/number and the two digit lists/maps (english and target) and returns the replaced string, then update convertToArabicNumbers and convertToUrduNumbers to convert input to string and delegate to this helper; ensure the helper is used by both functions and preserve current behavior and return types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/src/utils/utility.dart`:
- Around line 27-49: The digit-replacement loop in convertToArabicNumbers is
duplicated in convertToUrduNumbers; extract that shared logic into a private
helper (e.g., _convertDigits or _replaceDigits) that accepts the input
string/number and the two digit lists/maps (english and target) and returns the
replaced string, then update convertToArabicNumbers and convertToUrduNumbers to
convert input to string and delegate to this helper; ensure the helper is used
by both functions and preserve current behavior and return types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e7d39aa-e35f-4082-bf91-17aa3f252deb
📒 Files selected for processing (1)
lib/src/utils/utility.dart
nixrajput
left a comment
There was a problem hiding this comment.
Please check and fix failing test cases correctly
Pull Request Checklist
What does this PR do?
convertToArabicNumbers()utility function to correctly format Arabic time messagesChecklist
Code Changes
Documentation
pubspec.yamlfileTesting
General Tests
Localization
Custom Thresholds
Error Handling
Responsiveness
Performance
How did you verify your code works?
convertToArabicNumbers()function with various input numbers to ensure proper conversionSummary by CodeRabbit